-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Config.getini default none value #11282 #11496
Config.getini default none value #11282 #11496
Conversation
…ype string absent in INI file
for more information, see https://pre-commit.ci
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
…ype string absent in INI file
@@ -1525,6 +1525,8 @@ def _getini(self, name: str): | |||
return default | |||
if type is None: | |||
return "" | |||
if type == "string": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im very torn on this change in fear of the butterfly effec
at the same time and more propper solution will need a new api to avoid hell for users
@nicoddemus whats your impression on this
in particular the detail about the type is such, that for both None and string, it also makes sense to return a empty string
I just reported twmr/pytest-sphinx#60. Is that related? |
Closing this because @TanyaAgarwal28 hasn't been seen on GitHub for several months. Happy to reopen if it updates! |
PR Description:
This PR addresses the issue described in #11282 (replace with the issue number) where the pytest.config.getini() method returns an empty list instead of None when an INI option is not provided. This change ensures that None is returned as the default value when an INI option is not found or has no default value provided by the developer.
Changes Made:
Modified the getini method in config.py to return None as the default value when an INI option is not found or has no default value.
Added tests to cover this behavior, ensuring that None is returned when appropriate.
Testing:
Added unit tests to verify the expected behavior of the getini method.
Ran the test suite to ensure that existing functionality is not affected by this change.
Changelog:
Added a new feature to the config.py module: getini now returns None as the default value when an INI option is not provided or has no default value.
Checklist:
The code changes have been tested locally and work as expected.
Documentation has been updated to reflect the changes.
New tests have been added to cover the updated behavior.
Maintain the ability to push and squash commits when merging.
This PR addresses issue #11282.